Calendar graph #4419

akhileshsooji028 · 2024-01-24T10:11:39Z

⚠️⚠️⚠️ Since we do not accept all types of pull requests and do not want to waste your time. Please be sure that you have read pull request rules:
https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md#can-i-create-a-pull-request-for-uptime-kuma

Tick the checkbox if you understand [x]:

I have read and understand the pull request rules.

Description

Fixes #1984

Type of change

Please delete any options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
User interface (UI)
New feature (non-breaking change which adds functionality)
Breaking change (a fix or feature that would cause existing functionality to not work as expected)
Other
This change requires a documentation update

Checklist

My code follows the style guidelines of this project
I ran ESLint and other linters for modified files
I have performed a self-review of my own code and tested it
I have commented my code, particularly in hard-to-understand areas (including JSDoc for methods)
My changes generates no new warnings
My code needed automated testing. I have added them (this is optional task)

Screenshots (if any)

CommanderStorm

From the changes you made I am mostly not getting the why and think that this PR was created somewhat accidentally (?):

Why did you choose said libraries, was this not possible with chart.js?
You seem to not have used tippy.js. Is this intentional?
You commented out some code. Was this for testing purposes or is there a reason?
Please do fill out the description we provide. I have added the template.

akhileshsooji028 · 2024-01-24T10:42:08Z

In the Issue npm package calendar-graph was mentioned so the similar dedicated vue package has been used that is vue3-calendar-heatmap
Tippy.js is used to show the tooltip on hovering on the chart.

CommanderStorm

I have had the time to look over this PR and have made a few inline comments about some defects and other stuff.

In general:

to be mergable, the design needs to fit into the current design. Currently, you are using blue accents. Please try to keep the design close to our
Sorry if we were unclear about this part of the UI having to be configurable if significant changes like this one are made
Have you tested that this works as intended? See my concearns about the ´shortBeatList´.
While I like the way this looks, I am confused what a counts is. Please display normal metrics like daily uptime (in %) instead.

CommanderStorm · 2024-01-24T21:19:03Z

src/components/HeartbeatBar.vue

-            <div>{{ timeSinceFirstBeat }} ago</div>
-            <div v-if="$root.styleElapsedTime === 'with-line'" class="connecting-line"></div>
-            <div>{{ timeSinceLastBeat }}</div>
+            <CalendarHeatmap :style="{ fill: '#fff' }" class="heat-map" :values="values" :end-date="endDate" no-data-text="Down" tooltip-unit="counts" />


Here you removed the existing beat list.
Please make what is displayed configurable in the sidebars appreeance settings

src/components/PublicGroupList.vue

src/components/HeartbeatBar.vue

CommanderStorm · 2024-01-24T22:17:18Z

package.json

@@ -142,7 +142,9 @@
        "tar": "~6.1.11",
        "tcp-ping": "~0.1.1",
        "thirty-two": "~1.0.2",
+        "tippy.js": "^6.3.7",


A bit more context is needed on this one:
We already have popperjs installed.
Is there a reason why you chose this library?

Yes, the Vue-Calendar-Heatmap has suggested to use it as mentioned in the documentation.

Can we use popperjs instead for these popups?

CommanderStorm · 2024-01-26T17:10:29Z

In my opinion, we should drop this feature as the Current Graph stand with the standard of Uptime Kuma UI and one of the features which gives clean look to Uptime Kuma.

As long as this is configurable and is well integrated, such an alternate display mode is fine by me.
This is the design decision we chose for the last few features, which were design related and might be considered controversial.

CommanderStorm · 2024-01-27T16:04:29Z

by "alternate display mode" I mean that in the sidebar appearance settings a setting should be added that allows switching between the bigger calendar graph and the shorter beats graph.

CommanderStorm

This has improved, but not to the level we require.
These points from my previous review are not resolved.

CommanderStorm · 2024-02-06T22:51:46Z

server/routers/status-page-router.js

@@ -80,20 +80,32 @@ router.get("/api/status-page/heartbeat/:slug", cache("1 minutes"), async (reques
        ]);

        for (let monitorID of monitorIDList) {
+            // Calculating the percentage of uptime per day with the status count to show full year data


This is better (removing XY clounts), but still not what I meant when I said

in the sidebar appearance settings a setting should be added that allows switching between the bigger calendar graph and the shorter beats graph.

You switched all monitors to the calendar.
Please make this configurable either on

a per monitor (see the show link feature and the badge generator) or

per status page level.

CommanderStorm · 2024-02-06T22:53:59Z

server/routers/status-page-router.js

+                            ORDER BY COUNT(*) DESC
+                            LIMIT 1
+                        ) as status
+                    FROM heartbeat as main


@chakflying You have a better grasp at this (this is why the review-request).
Is this query correct? Should this not use stat_daily instead?

Yes, in the near future, the heartbeats table will only keep data for the last 48 hours. The getDataArray function in UptimeCalculator should be used instead. The raw counts data can retrieved that way, and the precentage calculated in the front-end.

Since this was only implemented recently, with the only component utilizing it (PingChart) still in PR state (#4264), any feedback on the implementation is welcome.

@chakflying correct me if i'm wrong, In near future ur gonna save only 48 hrs monitoring data right. so if I use getDataArray function, Will I get the last 365 days monitoring data?

The getDataArray function does not use the heartbeats table, and instead uses the various stats_ tables. You can take a look at the function implementation as it is in master. We have no time limit when using the 'day' interval.

getDataArray(num, type = "day") { if (type === "hour" && num > 24 * 30) { throw new Error("The maximum number of hours is 720"); } if (type === "minute" && num > 24 * 60) { throw new Error("The maximum number of minutes is 1440"); } ...

akhilesh.sooji added 3 commits January 24, 2024 15:23

Calendar Graph

8ca3e07

Calendar Graph - final

73aa1b5

Calendar Graph - final approach

ba2c9ea

CommanderStorm requested changes Jan 24, 2024

View reviewed changes

CommanderStorm marked this pull request as draft January 24, 2024 10:24

Calendar Graph - edits

f7b17b5

akhileshsooji028 marked this pull request as ready for review January 24, 2024 10:53

akhileshsooji028 marked this pull request as draft January 24, 2024 10:55

CommanderStorm requested changes Jan 24, 2024

View reviewed changes

CommanderStorm added the question Further information is requested label Jan 24, 2024

This comment was marked as resolved.

Sign in to view

CommanderStorm mentioned this pull request Jan 26, 2024

Calendar Graph #1984

Open

1 task

This comment was marked as resolved.

Sign in to view

added percentage marking heatmap

1fdeea0

akhileshsooji028 marked this pull request as ready for review February 6, 2024 08:05

akhileshsooji028 requested a review from CommanderStorm February 6, 2024 08:05

CommanderStorm requested review from chakflying and removed request for CommanderStorm February 6, 2024 22:46

CommanderStorm requested changes Feb 6, 2024

View reviewed changes

CommanderStorm marked this pull request as draft February 6, 2024 23:24

CommanderStorm added the area:status-page Everything related to the status page label Feb 12, 2024

CommanderStorm mentioned this pull request Mar 10, 2024

Support expanding group on status page to show sub-monitors / children #3324

Open

1 task

CommanderStorm added the needs:work this PR needs work label May 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Calendar graph #4419

Calendar graph #4419

akhileshsooji028 commented Jan 24, 2024 •

edited by CommanderStorm

CommanderStorm left a comment •

edited

akhileshsooji028 commented Jan 24, 2024 •

edited by CommanderStorm

CommanderStorm left a comment

CommanderStorm Jan 24, 2024

CommanderStorm Jan 24, 2024

akhileshsooji028 Jan 31, 2024

CommanderStorm Jan 31, 2024

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

CommanderStorm commented Jan 26, 2024

This comment was marked as resolved.

CommanderStorm commented Jan 27, 2024

CommanderStorm left a comment •

edited

CommanderStorm Feb 6, 2024

CommanderStorm Feb 6, 2024

chakflying Feb 7, 2024 •

edited

akhileshsooji028 Feb 28, 2024

chakflying Feb 28, 2024

Calendar graph #4419

Are you sure you want to change the base?

Calendar graph #4419

Conversation

akhileshsooji028 commented Jan 24, 2024 • edited by CommanderStorm

Description

Type of change

Checklist

Screenshots (if any)

CommanderStorm left a comment • edited

Choose a reason for hiding this comment

akhileshsooji028 commented Jan 24, 2024 • edited by CommanderStorm

CommanderStorm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

CommanderStorm commented Jan 26, 2024

This comment was marked as resolved.

CommanderStorm commented Jan 27, 2024

CommanderStorm left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chakflying Feb 7, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akhileshsooji028 commented Jan 24, 2024 •

edited by CommanderStorm

CommanderStorm left a comment •

edited

akhileshsooji028 commented Jan 24, 2024 •

edited by CommanderStorm

CommanderStorm left a comment •

edited

chakflying Feb 7, 2024 •

edited